feat(azdext): Extension SDK improvements — integration, security, runtime, cleanup#7025
feat(azdext): Extension SDK improvements — integration, security, runtime, cleanup#7025jongio wants to merge 9 commits intoAzure:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR extends the azdext Extension SDK with additional integration, security, output/logging, and runtime helper utilities, and updates docs/extensions to use the improved entrypoints and patterns.
Changes:
- Add new
azdexthelper modules: TUI/interactive detection, tool discovery & PATH management, shell execution, atomic file operations, process utilities, structured output, structured logging, Key Vault secret resolution, SSRF guard, and input validation helpers. - Harden/adjust existing primitives (pager, scope detector, resilient HTTP client) and expand tests across the new surfaces.
- Update extensions and docs to use
azdext.Runand document new SDK helper guidance.
Reviewed changes
Copilot reviewed 47 out of 51 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| cli/azd/pkg/azdext/tui.go | Adds interactive capability detection helpers. |
| cli/azd/pkg/azdext/tui_test.go | Unit tests for interactive detection and flags. |
| cli/azd/pkg/azdext/tooldiscovery.go | Adds tool lookup + PATH management utilities. |
| cli/azd/pkg/azdext/tooldiscovery_test.go | Tests for tool lookup and PATH helpers. |
| cli/azd/pkg/azdext/shell.go | Adds shell detection + shell command helpers + TTY detection. |
| cli/azd/pkg/azdext/shell_test.go | Tests for shell helpers and terminal detection. |
| cli/azd/pkg/azdext/security_validation.go | Adds validation helpers (service name, hostname, script name, container env detection). |
| cli/azd/pkg/azdext/security_validation_test.go | Tests for validation and container env detection. |
| cli/azd/pkg/azdext/ssrf_guard.go | Adds standalone SSRF guard utility (non-MCP usage). |
| cli/azd/pkg/azdext/ssrf_common.go | Common SSRF constants + IP classification helpers. |
| cli/azd/pkg/azdext/process.go | Adds cross-platform process utilities API surface. |
| cli/azd/pkg/azdext/process_windows.go | Windows implementation for process utilities. |
| cli/azd/pkg/azdext/process_darwin.go | macOS implementation for process utilities. |
| cli/azd/pkg/azdext/process_linux.go | Linux implementation for process utilities. |
| cli/azd/pkg/azdext/process_test.go | Tests for process utilities. |
| cli/azd/pkg/azdext/atomicfile.go | Adds atomic write/copy/backup + EnsureDir helpers. |
| cli/azd/pkg/azdext/atomicfile_test.go | Tests for atomic file helpers. |
| cli/azd/pkg/azdext/output.go | Adds format-aware output helper (default vs JSON) + table rendering. |
| cli/azd/pkg/azdext/output_test.go | Tests for output helper behavior. |
| cli/azd/pkg/azdext/logger.go | Adds slog-based logger + global setup helper. |
| cli/azd/pkg/azdext/logger_test.go | Tests for logger levels, structured output, and chaining. |
| cli/azd/pkg/azdext/keyvault_resolver.go | Adds Key Vault akvs:// secret reference resolver. |
| cli/azd/pkg/azdext/mcp_server_builder.go | Adds note about potential future package split for MCP library coupling. |
| cli/azd/pkg/azdext/extension_command.go | Adds note about potential future package split for Cobra coupling. |
| cli/azd/pkg/azdext/context.go | Deprecates NewContext in favor of Run/command helpers. |
| cli/azd/pkg/azdext/token_provider.go | Updates TokenProvider doc snippet (signature change). |
| cli/azd/pkg/azdext/scope_detector.go | Adjusts rule handling + error formatting behavior. |
| cli/azd/pkg/azdext/resilient_http_client.go | Adjusts retry defaults, jitter, and Retry-After parsing behavior. |
| cli/azd/pkg/azdext/resilient_http_client_test.go | Updates tests for resilient client behavior changes. |
| cli/azd/pkg/azdext/pagination.go | Adjusts pager init/validation and truncation semantics. |
| cli/azd/pkg/azdext/pagination_test.go | Updates tests for pager behavior changes. |
| cli/azd/pkg/azdext/run_test.go | Tests for error message/suggestion extraction helpers. |
| cli/azd/extensions/azure.appservice/main.go | Switches extension entrypoint to azdext.Run. |
| cli/azd/extensions/azure.appservice/internal/cmd/swap.go | Adds selection index bounds checks + slot name validation. |
| cli/azd/extensions/azure.appservice/go.mod | Updates module deps/replace comment for monorepo development. |
| cli/azd/extensions/azure.appservice/go.sum | Updates dependency checksums. |
| cli/azd/extensions/azure.ai.agents/internal/pkg/agents/agent_api/models.go | Formatting fix (go fmt). |
| cli/azd/extensions/azure.ai.agents/internal/cmd/show.go | Formatting fix (alignment). |
| cli/azd/extensions/azure.ai.agents/internal/cmd/monitor.go | Formatting fix (alignment). |
| cli/azd/docs/extensions/extension-framework.md | Updates docs to recommend azdext.Run and links related guides. |
| cli/azd/CHANGELOG.md | Changelog entries for new SDK helper surfaces and docs. |
| cli/azd/.vscode/cspell.yaml | Adds dictionary words for new SDK terminology. |
Comments suppressed due to low confidence (1)
cli/azd/pkg/azdext/pagination.go:234
validateNextLinkallowsnextLinkvalues with an empty scheme/host (e.g. relative URLs like/page2) and will accept them without SSRF host validation. This will later fail inhttp.NewRequest/Do() (missing scheme/host) and is also inconsistent with the comment that nextLink “must stay on the same host with HTTPS”. Consider requiring an absolute HTTPS URL (or explicitly resolving relative URLs against the current/origin URL and then enforcing same-host + https).
// validateNextLink checks that a nextLink URL is safe to follow.
// It rejects non-HTTPS schemes, URLs with embedded credentials, and
// URLs pointing to a different host than the original request (SSRF protection).
func (p *Pager[T]) validateNextLink(nextLink string) error {
u, err := url.Parse(nextLink)
if err != nil {
return fmt.Errorf("invalid nextLink URL: %w", err)
}
if u.Scheme != "" && u.Scheme != "https" {
return fmt.Errorf("nextLink must use HTTPS (got %q)", u.Scheme)
}
if u.User != nil {
return errors.New("nextLink must not contain user credentials")
}
host := strings.ToLower(u.Hostname())
if host != "" && p.originHost != "" && host != p.originHost {
return fmt.Errorf("nextLink host %q does not match origin host %q (possible SSRF)", host, p.originHost)
}
return nil
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
9554fed to
fa5b1f7
Compare
|
@copilot please re-review the latest changes. All 6 agreed-upon review comments have been addressed. |
fa5b1f7 to
6603758
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 47 out of 51 changed files in this pull request and generated 10 comments.
Comments suppressed due to low confidence (1)
cli/azd/pkg/azdext/pagination.go:193
- Pager.NextPage reads the response body with io.LimitReader(resp.Body, maxPageResponseSize) and then unmarshals it. If the server returns more than maxPageResponseSize bytes, the body will be silently truncated and the JSON decode error will be misleading. Consider restoring the prior pattern of reading maxPageResponseSize+1 and returning an explicit "response exceeds max page size" error when the limit is exceeded.
data, err := io.ReadAll(io.LimitReader(resp.Body, maxPageResponseSize))
if err != nil {
return nil, fmt.Errorf("azdext.Pager.NextPage: failed to read response: %w", err)
}
var page PageResponse[T]
if err := json.Unmarshal(data, &page); err != nil {
return nil, fmt.Errorf("azdext.Pager.NextPage: failed to decode response: %w", err)
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Architecture Note: KeyVault Resolver Design DecisionThe What IS shared (no duplication):
What is NOT shared (by design):
Core keyvault requires Azdext resolver accepts a simple If we consolidate in the future, the azdext pattern is the better foundation — simpler credential model, better error handling, and inherently testable. Core's |
|
I will pull the key vault resolution feature into a new pr. |
jongio
left a comment
There was a problem hiding this comment.
blockMetadata IS read — it's used at lines 229 and 252 of checkCore() to classify blocked metadata hosts with the distinct metadata_endpoint reason instead of the generic blocked_host. The field, the documented SSRFError.Reason values, and the implementation are all aligned. No changes needed.
01aaaea to
564e281
Compare
Azure Dev CLI Install InstructionsInstall scriptsMacOS/Linux
bash: pwsh: WindowsPowerShell install MSI install Standalone Binary
MSI
Documentationlearn.microsoft.com documentationtitle: Azure Developer CLI reference
|
…time, cleanup Consolidated PR covering issues Azure#6945, Azure#6946, Azure#6947, Azure#6948, Azure#6949: - Key Vault resolver + config helpers (Azure#6945) - Output + structured logging helpers (Azure#6946) - Security validation + SSRF guard (Azure#6947) - Runtime utilities: shell, file, process, TUI, tool discovery (Azure#6948) - Post-Azure#6856 cleanup: context ownership, package boundaries (Azure#6949) Fixes Azure#6945 Fixes Azure#6946 Fixes Azure#6947 Fixes Azure#6948 Fixes Azure#6949
… resolver - Add nil-check guards to process utilities - Fix shell detection edge cases - Simplify pagination with bounds checks - Harden SSRF guard with additional blocked ranges - Expand KeyVault resolver with @Microsoft.KeyVault format support - Add extensions.go and show.go improvements - Add keyvault.go helper functions for reference parsing Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Remove KeyVaultResolver, its tests, and keyvault.go helper additions from this PR. The KV resolution feature is being tracked separately as it is too large and contentious to bundle with other SDK improvements. The feature will be submitted in a dedicated branch and PR. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Fix swap.go indentation (extra tab on idx assignment) - Reject relative nextLink URLs in pagination (SSRF protection) - Clarify sanitizeControlChars comment - Use golang.org/x/term.IsTerminal for reliable TTY detection Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Add 9 words to cspell dictionary (Fprintf, myhost, IMDS, sctx, Println, exfiltration, gocritic, dockerenv, preconfigured) - Add //nolint:gosec annotations for G703, G204, G115 false positives in atomicfile.go, shell.go, and tooldiscovery.go Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- SSRF: Replace ad-hoc IP checks with comprehensive ssrfCheckIP() blocklist - SSRF: Document DNS rebinding TOCTOU limitation with TODO - Security: Case-insensitive path comparison on Windows - Pagination: Clear error on response truncation instead of cryptic JSON fail - API: Make SkipExpand unexported sentinel with IsSkipExpand() accessor - Concurrency: Fix worker goroutine deadlock risk in ARM resource walker - Container apps: Match target container by image repo instead of index 0 - Dedup: Fall back to ResourceName when ResourceID is empty - Tests: Add AZD_DEBUG fallback tests, remove unsafe os.Unsetenv - Cleanup: Deduplicate env expansion, use strconv.ParseBool consistently - Docs: Security notes for local-before-PATH and dotted path rejection Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Add missing words to cspell dictionary: dedup, gradlew, managedhsm, microsoftazure, newtag, TTLs - Remove unused checkIPEncodingVariants function flagged by golangci-lint Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
e5d5371 to
f35035d
Compare
- Fix unused jitter variable in resilient_http_client.go - Fix pagination_test.go: add fmt import, fix unused vars, fix body reference - Apply go fix: use min() builtin, maps.Copy, range-over-int Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Extension SDK Improvements — Integration, Security, Runtime, Cleanup
Summary
Consolidated PR delivering the remaining Extension SDK and MCP Framework improvements from the Extension Framework Improvements initiative. These features were identified through a multi-model code review across 5 production extensions (azd-app, azd-exec, azd-copilot, azd-rest) and a shared library (azd-core), revealing ~2,500-4,000 lines of duplicated infrastructure across the ecosystem.
Changes
P1 Config Helpers (#6945)
Config Helpers
~/.azd/config.jsonwith defaults. Each extension builds its own config loader.config_helper.go—ConfigHelperwith typed loading, saving, defaults, and atomic writes.config_helper_test.go— 967 lines of comprehensive tests.P2 Output & Structured Logging (#6946)
Output Helpers
Success(),Error(),Warning(),Table(),SetFormat()— used by azd-app and all other extensions. Without framework support, every extension depends on azd-core for consistent output.output.go—Outputhelper with format-aware rendering (text, JSON, table), ANSI color support.output_test.go— 358 lines of tests.Structured Logger
SetupLogger(debug, structured)with JSON/text format selection and component-scoped loggers.logger.go—SetupLoggerandComponentLoggerwith slog integration.logger_test.go— 295 lines of tests.P2 Security Validation & SSRF Guard (#6947)
Security Validation
ValidatePath()(L33 — detects.., resolves symlinks),ValidateServiceName()(L85 — DNS-safe regex),SanitizeScriptName()(L129 — blocks shell metacharacters), andIsContainerEnvironment()(L148 — detects Docker, Codespaces, K8s).security_validation.go—ValidatePath,ValidateServiceName,SanitizeScriptName,IsContainerEnvironment.security_validation_test.go— 354 lines of tests.SSRF Guard
blockedHeaders(L34),blockedHostsincluding169.254.169.254andfd00:ec2::254(L42), 7blockedCIDRsfor IPv4/IPv6 loopback, link-local, and RFC 1918 (L49-66), plusisBlockedIP()(L84) andisBlockedURL()with DNS resolution (L96-134).ssrf_guard.go+ssrf_common.go—SSRFGuardwith fluent builder pattern, metadata endpoint blocking, private network blocking, HTTPS enforcement, DNS resolution, IPv6 embedded IPv4 extraction, OnBlocked callback.ssrf_guard_test.go— 552 lines of tests. Also refactoredmcp_security.goto use shared SSRF primitives.P3 Runtime Utilities (#6948)
Shell Detection & Execution
DetectShell(). azd-exec duplicates shell argument building in its MCP handler separately from its CLI handler.shell.go—DetectShell,ShellInfo,BuildCommand,IsInteractiveTerminal(usinggolang.org/x/term), shell type constants.shell_test.go— 258 lines of tests.Atomic File Operations
AtomicWriteJSON()andAtomicWriteFile()— write to temp, sync, set permissions, rename.atomicfile.go—AtomicWriteFile,AtomicWriteJSONwith platform-aware temp file placement.atomicfile_test.go— 322 lines of tests.Process Management
os.FindProcessisn't reliable.gopsutilfor accurate detection across Windows/Linux/macOS.process.go+ platform-specific files (process_windows.go,process_linux.go,process_darwin.go) —IsProcessRunning,GetProcessInfo,WaitForProcessExitusing native OS APIs (Windows:OpenProcess+QueryFullProcessImageNameW, Linux:/proc, macOS:ps). EPERM handling on Unix for permission-denied processes.process_test.go— 221 lines of tests.TUI Helpers
SetStdHandle()for CONIN$/CONOUT$, Unix/dev/ttybypass.tui.go—LaunchTUIwith platform-specific stdin/stdout reattachment.tui_test.go— 240 lines of tests.Tool Discovery
FindToolInPath()with Windows .exe handling andGetInstallSuggestion()for 18+ tools.tooldiscovery.go—FindTool,GetInstallSuggestion,RequireTool,BuildPATH.tooldiscovery_test.go— 268 lines of tests.Cleanup (#6949)
Context Ownership & Package Boundaries
context.go— additional context helpers.extension_command.go— package boundary clarification.scope_detector.go— custom rule precedence fix (custom rules now prepend before defaults).token_provider.go— simplified.mcp_server_builder.go— minor additions.Resilient HTTP Client Improvements
resilient_http_client.go— bounded body drain (1 MiB via io.LimitReader), improved defaults handling.resilient_http_client_test.go— updated tests.Pagination Improvements
pagination.go—sanitizeControlCharsfor error bodies,validateNextLinknow rejects relative URLs and non-HTTPS schemes.pagination_test.go— updated tests.Documentation
extension-e2e-walkthrough.md— 477-line end-to-end guideextension-migration-guide.md— 554-line migration guide from azd-coreextension-sdk-reference.md— 705-line SDK referenceextension-framework.md— updated with new capabilitiesExample Extension Update
extensions/azure.appservice/— updated to use SDK helpers, demonstrating the migration pattern.Related Work
Downstream Impact
WalktoWalkDir#22): Deprecated stopgap helpers in favor of SDK-provided onesTesting
Fixes #6945, Fixes #6946, Fixes #6947, Fixes #6948, Fixes #6949